Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Street geo editor #540

Merged
merged 65 commits into from
Jun 7, 2024
Merged

Street geo editor #540

merged 65 commits into from
Jun 7, 2024

Conversation

Algorush
Copy link
Collaborator

@Algorush Algorush commented May 30, 2024

first version for this issue: #526
Implemented all from issue list except hide the properties panel for entities with "autocreated" class.
There is an open question about adding an elevation field.
I also need to add a change Marker position on the map when entering location manually in the location field (new location or current location?).
And also I forgot that I need to move geo icon somewhere or help icon. Because now they overlap

Copy link

netlify bot commented May 30, 2024

👷 Deploy Preview for 3dstreet-core-builds processing.

Name Link
🔨 Latest commit 5205ac3
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/666317c23ab1fe0008297562

@kfarr
Copy link
Collaborator

kfarr commented May 30, 2024

Thanks for the start

  • * The current location of the geospatial long/lat should display in the geomodal if it is available
  • * searching for location should work as expected
  • * the design of main.js footer section should roughly match: restyle of footer buttons and title #589 (note the relocated help icon; here is figma link: https://www.figma.com/design/W9jid3A0jgssnIKBFEvG38/3DStreet-2022?node-id=6555-175608&t=tzhDzsJib77eAp8r-4)
  • * Coordinate field handler to allow user to enter coordinates and change map
  • * ?Location label in geopanel: shows as "SF, CA at Market and Van Ness" as static. We could instead show longitude and latitude? If no location then "Click to set location". Clicking on location text can be same action as clicking on icon
  • * ?add an elevation number entry field beneath centerpoint lat/lon field?

@Algorush
Copy link
Collaborator Author

Algorush commented May 31, 2024

Another question is make the coordinate field read-only or add a handler to allow user to enter coordinates and change the map? Because now an error appears when changing coordinates via input

@Algorush
Copy link
Collaborator Author

Error while saving

"Error trying to save 3DStreet scene to cloud. Error: RangeError: Maximum call stack size exceeded"

If add 'reference-layers' to the skipChildrenNodes array in json-utils.js in the getElementData function, its children will not be saved in JSON. This fixes a stack overload bug when saving for now.
But I understand that the reason is different, of course

Algorush and others added 4 commits May 30, 2024 23:39
match google tiles 3d rotation
fix zoom
add SF lat/long default, satellite view zoomed in by default
@Algorush
Copy link
Collaborator Author

Algorush commented Jun 5, 2024

Ok. It looks like it all working now. I just noticed that the aspect ratio of the camera in Plan View does not adapt to the screen. I need to fix this.

@kfarr
Copy link
Collaborator

kfarr commented Jun 5, 2024

Ok. It looks like it all working now.

It looks like the geopanel does not display the long/lat/elevation as it did before. I testing other stuff to see if it's all working

I just noticed that the aspect ratio of the camera in Plan View does not adapt to the screen. I need to fix this.

Yes, although I think this was already a solved problem in the a-frame inspector so there may be an easy fix to port over. 3DStreet/3dstreet-editor#185

kfarr added 4 commits June 4, 2024 20:49
* update submodule for dist latest

to add: waymo, uoregon objects, light rail vehicle size update

* add u of o objects

3DStreet/3dstreet-assets-source#84

* add waymo

* fix tram path for updated size

* update submodule for trash truck

* trash truck
@Algorush
Copy link
Collaborator Author

Algorush commented Jun 5, 2024

It looks like the geopanel does not display the long/lat/elevation as it did before. I testing other stuff to see if it's all working

thanks, I forgot about this. Yesterday I spent half a day debugging the work of 3dtiles + Editor. Was focused on this.

@kfarr
Copy link
Collaborator

kfarr commented Jun 6, 2024

@Algorush did testing today and the only "showstopper" issue I found was elevation is a confusing thing for user to edit directly, instead it would be helpful to provide that value on behalf of the user.

In this glitch project there is an example of vanilla js method using ElevationService to get elevation for a given lat / lon. I would suggest that we do the same thing -- when a user clicks on a place on the map, the elevation is updated based on that lat/lon.

https://glitch.com/edit/#!/bollard-buddy-qr-maker?path=script.js%3A43%3A0

    // Get the elevation at the clicked location    
    const elevator = new google.maps.ElevationService();
    elevator.getElevationForLocations({
      locations: [mapsMouseEvent.latLng],
    }, function(elevationResponse, status) {
      if (status === 'OK' && elevationResponse && elevationResponse.length > 0) {
        const elevation = elevationResponse[0].elevation;
        console.log('Elevation:', elevation);
        // Use the elevation value as needed...

@Algorush
Copy link
Collaborator Author

Algorush commented Jun 6, 2024

@Algorush did testing today and the only "showstopper" issue I found was elevation is a confusing thing for user to edit directly, instead it would be helpful to provide that value on behalf of the user.

In this glitch project there is an example of vanilla js method using ElevationService to get elevation for a given lat / lon. I would suggest that we do the same thing -- when a user clicks on a place on the map, the elevation is updated based on that lat/lon.

Thanks! I didn't know about this feature on google maps. I'll get it to set elevation.
So, do this - set the elevation level using this request to google.maps.elevationService. But leave the elevation field so that the user can change it?

@kfarr
Copy link
Collaborator

kfarr commented Jun 6, 2024

So, do this - set the elevation level using this request to google.maps.elevationService. But leave the elevation field so that the user can change it?

Yes, in other words if a user chooses a new lat / long, then overwrite any existing elevation value with the new elevation value from the ElevationService in the modal dialog. Of course, the user still may choose to cancel and their existing scene elevation is not overwritten, but I assume that if they choose a new location they will want the elevation calculated automatically, at least at first.

kfarr and others added 10 commits June 6, 2024 13:57
causing local runtime errors
reduce confusion for users wishing to create a new scene
* add viewer param, hide viewer sidebar

* hide sky on enter ar

* Squashed commit of the following:

commit 403f97e
Author: Kieran Farr <[email protected]>
Date:   Tue Jun 4 21:39:05 2024 -0700

    prevent scrolling in geomodal

commit d71a7b5
Author: Kieran Farr <[email protected]>
Date:   Tue Jun 4 21:14:07 2024 -0700

    move search input below map

commit f26ec38
Author: Kieran Farr <[email protected]>
Date:   Tue Jun 4 21:12:41 2024 -0700

    update geopanel coords from entity instead of metadata

commit a4afc3a
Author: Kieran Farr <[email protected]>
Date:   Tue Jun 4 20:49:48 2024 -0700

    Add spring objects (#559)

    * update submodule for dist latest

    to add: waymo, uoregon objects, light rail vehicle size update

    * add u of o objects

    3DStreet/3dstreet-assets-source#84

    * add waymo

    * fix tram path for updated size

    * update submodule for trash truck

    * trash truck

commit ff551ce
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 22:55:20 2024 -0400

    update three-loader-3dtiles lib version

commit d1cbe64
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 22:54:00 2024 -0400

    remove old line to work 3dtiles with editor

commit c7a79fe
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 22:52:33 2024 -0400

    dynamically load aframe-loader-3dtiles-component, update code for it

commit c9a1a03
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 20:37:50 2024 -0400

    new aframe-loader-3dtiles dist
    set Editor camera if its active on init
    update lat,long and/or height in runtime

commit 2a04971
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 20:15:38 2024 -0400

    don't store geo coords in metadata. Store it in street-geo

commit 43d7714
Merge: abee710 d15bd99
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 14:12:32 2024 -0400

    Merge branch 'street-geo-editor' of https://github.com/3DStreet/3dstreet into street-geo-editor

commit abee710
Author: Alexander Goryushkin <[email protected]>
Date:   Tue Jun 4 14:12:00 2024 -0400

    get GOOGLE_API_KEY from firebaseConfig.apiKey

commit d15bd99
Author: Vincent Fretin <[email protected]>
Date:   Tue Jun 4 18:24:26 2024 +0200

    remove the whole dist folder and add it to gitignore

commit 695418a
Merge: b148095 5b9dc54
Author: Alexander Goryushkin <[email protected]>
Date:   Mon Jun 3 23:58:12 2024 -0400

    Merge branch 'street-geo-editor' of https://github.com/3DStreet/3dstreet into street-geo-editor

commit b148095
Author: Alexander Goryushkin <[email protected]>
Date:   Mon Jun 3 23:57:24 2024 -0400

    save/load geoData in JSON file

* merge assets too

* add pro badge to geo

this should be visible but disabled for non-pro users

* remove unneeded icons file

* fix autocomplete styling

* assets submodule update

* qr-code-ui-wip

next - upon click, then download png of qr code (or open in new tab)

* wip qrhandler

need qr code

* Squashed commit of the following:

commit c1b4c26
Author: Kieran Farr <[email protected]>
Date:   Thu Jun 6 22:29:31 2024 -0700

    pro user required for cloud save with long/lat

commit 165866a
Author: Kieran Farr <[email protected]>
Date:   Thu Jun 6 22:12:17 2024 -0700

    untitled instead of new scene

    reduce confusion for users wishing to create a new scene

commit e27b8cd
Author: Kieran Farr <[email protected]>
Date:   Thu Jun 6 22:11:52 2024 -0700

    wHoops

* do not load geo upon ar mode

why not just hide geospatial layers upon ar mode instead of preventing their load? because some of them are super heavy and can crash mobile devices

* update submodules and such
@kfarr kfarr merged commit 1cd556f into main Jun 7, 2024
1 of 5 checks passed
@kfarr kfarr deleted the street-geo-editor branch June 7, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants